Skip to content

Add helpText for SpliceAI#3637

Merged
bpblanken merged 5 commits intodevfrom
benb/3579
Sep 29, 2023
Merged

Add helpText for SpliceAI#3637
bpblanken merged 5 commits intodevfrom
benb/3579

Conversation

@bpblanken
Copy link
Collaborator

Screenshot 2023-09-27 at 5 01 15 PM

@hanars
Copy link
Collaborator

hanars commented Sep 28, 2023

Sorry, I should have checked that this ticket was clearer before assigning it over to you. When the ticket says we should show it "similar to how we show it for the Pathogenicity override", it means we should show it directly in the panel above all the filters, not on a hover. The text we should use should be something like "Filter by reported annotation. Variants will be returned if they have ANY of the specified annotations, including if they have a Splice AI score above the threshold and no other annotations. This filter is overridden by the pathogenicity filter, so variants will be returned if they have the specified pathogenicity even if none of the annotation filters match"

Screenshot 2023-09-28 at 8 35 36 AM

@bpblanken
Copy link
Collaborator Author

👍 Do we also want to mention "screen"? It has the same behavior as splice_ai I believe?

@hanars
Copy link
Collaborator

hanars commented Sep 28, 2023

I feel like screen is covered by "ANY of the specified annotations", it pretty much looks to users like thr other annotations in the filter so while in the database the filtering is quite different, to the user I do not think it is

@bpblanken bpblanken reopened this Sep 28, 2023
@bpblanken
Copy link
Collaborator Author

Updated screenshot:
Screenshot 2023-09-28 at 2 54 59 PM

const labelHelp = (
<div>
{`Enter a numeric cutoff for ${label}`}
{dangerThreshold && (
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is dead code after the recent changes... is that correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but actually we should be replacing this with updated logic from the config thresholds, not just removing it. I would leave it for now and make a ticket to properly update it (or you can give that a go if you want)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants